[WA-3252] Show CGW errors clearly: map known response states to agreed UI copy - #8592
[WA-3252] Show CGW errors clearly: map known response states to agreed UI copy#8592Clóvis Neto (clovisdasilvaneto) wants to merge 3 commits into
Conversation
A CGW 502 answered with an HTML error page was rendered verbatim to the user while submitting a transaction. RTK Query cannot JSON-parse that body, so it reports PARSING_ERROR and `asError` hoisted the raw HTML into `error.message`, which the notification toast prints inside a <pre> and the inline submit error prints in its Details panel. There was also no agreed contract for what the UI shows on a known CGW response, so codes were handled ad hoc or not at all. - Add `gatewayErrors.ts` next to `contractErrors.ts`: one shared, code-keyed source of copy for 422 / 429 / 451 / any 5xx, read by web and mobile. 404 is deliberately left unmapped. - Stop `asError` surfacing an unparsable or markup response body as the message, and keep the real HTTP status (`originalStatus`) so the UI can map it. `getHttpStatusFromError` now reads `originalStatus` too. - Render the agreed copy in `TxSubmitError` and in the transaction and safe-message notification hooks; show the code-only support reference (`CGW-502`) in Details instead of the raw payload. - Alert internally on a 422 (a malformed request is our bug) from a single RTK Query middleware. Only the numeric status reaches analytics. Nothing retries CGW requests, so a 422 is surfaced once; a test pins that. A retry policy for the transient states (429 / 5xx) is deferred to its own ticket, since it cannot be delivered for both platforms without changing mobile's test harness.
tx-builder Preview✅ Deploy successful! Preview URL: |
📦 Next.js Bundle Analysis for @safe-global/webThis analysis was generated by the Next.js Bundle Analysis action. 🤖 🎉 Global Bundle Size Decreased
DetailsThe global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster. Any third party scripts you have added directly to your app using the If you want further insight into what is behind the changes, give @next/bundle-analyzer a try! Twenty-nine Pages Changed SizeThe following pages changed size from the code in this PR compared to its base branch:
DetailsOnly the gzipped size is provided here based on an expert tip. First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If Any third party scripts you have added directly to your app using the Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. |
Coverage report for
|
St.❔ |
Category | Percentage | Covered / Total |
|---|---|---|---|
| 🟢 | Statements | 85.16% (+0% 🔼) |
35450/41627 |
| 🟡 | Branches | 70.28% (-0.03% 🔻) |
11605/16513 |
| 🟡 | Functions | 73.59% (+0.04% 🔼) |
5405/7345 |
| 🟢 | Lines | 86.4% (-0% 🔻) |
31721/36716 |
Show new covered files 🐣
St.❔ |
File | Statements | Branches | Functions | Lines |
|---|---|---|---|---|---|
| 🟢 | ... / cgwErrorAlert.ts |
100% | 100% | 100% | 100% |
| 🟢 | ... / cgw-errors.ts |
100% | 100% | 100% | 100% |
| 🟢 | ... / useTxNotifications.ts |
78.89% | 53.13% | 90% | 80% |
Test suite run success
7812 tests passing in 888 suites.
Report generated by 🧪jest coverage report action from a45231a
…WA-3252) A 429-carrying error matches both the viem rate-limit classifier and the CGW HTTP-status map. `TxSubmitError` checked rate-limit first, but `useTxNotifications` checked the CGW branch first, so one and the same failure rendered "Network is busy. Please try again in a moment." inline and "Something went wrong on our end. Try again." on the toast. Move the `cgwError` branch after the `isRateLimitError` branch so both surfaces resolve the overlap identically. The CGW support reference (`Error code CGW-429`) is unchanged and still accompanies the toast, matching the inline alert's code-only reference. Tests pin the ordering on both surfaces; swapping the branches back fails them. Also sharpen two tests that could not fail for the reason their names claimed, and correct the scope comment on the CGW retry pin: it covers `dynamicBaseQuery` only, not the `retry()` wrapper in `gateway/chains/index.ts`. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ybxq8EGntf1FmNC7Q2kLz1
2 participants
What it solves
Resolves: WA-3252
A 502 from CGW rendered to users as raw HTML while submitting a transaction (reported by Liliya). The underlying problem is bigger than one status code: there was no agreed contract for what the UI shows on known CGW responses, so codes were handled ad hoc or not at all.
Where the HTML actually escaped
Not at the render layer — two layers earlier, in shared code.
RTK Query cannot JSON-parse a gateway's HTML error page, so
fetchBaseQueryreturns{ status: 'PARSING_ERROR', originalStatus: 502, data: '<html>…502 Bad Gateway…nginx…' }. Inpackages/utils/src/services/exceptions/utils.ts,asError's branch order hittypeof thrown.data === 'string'first and hoisted the entire HTML body intoerror.message, whileerror.statusbecame the string'PARSING_ERROR'— so the real 502 was discarded andgetHttpStatusFromErrorreturnedundefined.From there it surfaced verbatim in two places:
proposeTransaction→TxEvent.PROPOSE_FAILED→useTxNotifications→detailedMessagerendered inside a<pre>; and the same error object intoTxSubmitError→ErrorMessagedetails.Fixing it in
asErrorcloses the leak for every consumer at once — including mobile, which shares that function.How this PR fixes it
Shared (
packages/)exceptions/gatewayErrors.ts(new) — the code-keyed copy source, sitting besidecontractErrors.tsand shaped like it. 422 / 429 / 451 explicit plus a 5xx range rule.exceptions/utils.ts—asErrorno longer surfaces an unparsable or markup body as the message, and preservesoriginalStatusas a numericstatus;getHttpStatusFromErrornow also readsoriginalStatus.exceptions/ErrorCodes.ts/errorTaxonomy.ts— one line each (_622), sologErrorhas a code to take and the WA-2775 facets don't degrade totype: unknown.Web (
apps/web/)utils/cgw-errors.ts(new) —getCgwErrorInfo/getCgwSupportCode; every component edit calls into it rather than spreading conditionals.ErrorMessage— a known CGW state showsError code CGW-502with a copy button instead of a Details toggle over the payload.TxSubmitError— one self-contained branch after the rate-limit branch.useTxNotifications/useSafeMessageNotifications— the mapped sentence, anddetailedMessage: undefinedfor CGW errors.store/middleware/cgwErrorAlert.ts(new) — the 422 internal alert.The agreed contract
A 422 means we sent CGW a malformed request — our bug, not the user's — so it emits an internal alert via
logError(Errors._622, …)→logger.warn+captureError({ isUserFacing: false }), i.e. the Datadog debugging sink.On analytics
trackErrorSurfacedandnormalizeErrorare untouched. What reaches Mixpanel is only what it already sent: normalized enums plushttp_status, a number already whitelisted inmapContext. No response body, no message text, no new property. The WA-2775 privacy invariant ("the sanitized message stays out of Mixpanel") is intact.Deliberately out of scope
Three scope decisions, all deliberate:
404with the same response shape. Guessing would either suppress real errors or pollute normal empty states, so it is deferred to a follow-up.gatewayErrors.tsleaves 404 unmapped, with tests pinning that, and existing 404 suppression (useLoadSafeInfo'sisCgw404) is untouched.retry()on the sharedcgwClientbase query cannot satisfy both platforms without changing mobile's test harness (jest.setup.tsxcallsjest.useFakeTimers()globally, so a backoffsetTimeoutnever fires). A web-only retry would ship a silently asymmetric AC, so retry moves to its own ticket.Correction on the retry claim
An earlier revision of this description said "nothing retries CGW requests today". That is accurate for the submission flow this ticket fixes, but not repo-wide:
packages/store/src/gateway/chains/index.ts:16wrapsdynamicBaseQueryinretry(…, { maxRetries: 5 })with noretryCondition, so failures on the chains endpoints are re-sent. The propose/submission path genuinely has no retry, which is why AC4 ("422 does not retry-loop") holds for the defect flow.cgwClient-no-retry.test.tspins the base query directly and therefore covers neither the chains wrapper nor aretry()added atcreateApilevel — its comment has been corrected to say so rather than overclaim.How to test it
Manual — intercept the CGW request (Requestly, as QA does) and return each status while submitting a transaction:
nginx, no status line.Affected flows
ErrorMessage,TxSubmitError, or the tx/message notification toastsBlast radius
packages/utils/.../exceptions/utils.ts(asError)error.message; the real HTTP status is preserved. Mobile's full suite was run and passes.packages/utils/.../gatewayErrors.tsErrorCodes.ts/errorTaxonomy.tscomponents/tx/ErrorMessage,TxSubmitErrorhooks/useTxNotifications,useSafeMessageNotificationsdetailedMessagewithheld for CGW errors onlystore/index.tsNot touched:
contractErrors.ts,trackErrorSurfaced,normalizeError, the Mixpanel payload, mobile source, CI, dependencies.apps/tx-builderdoes not depend on@safe-global/utilsand is unaffected despite its preview job running.Risks / not checked
PARSING_ERRORshape, not an end-to-end request against a failing gateway.asErroris shared with mobile. Its full suite passes (353 suites / 2930 tests) and mobile source is untouched, but the mobile app was not run.isMarkupis/^\s*</. A plain-text error body that legitimately starts with<would be treated as markup and replaced with "Request failed with status N" — informative, but not the original text.hooks/__tests__/useTxNotifications.test.ts. The two are semantically independent — Ledger errors carry no HTTP status, so the CGW classifier cannot false-positive on them — but whichever merges second needs a mechanical conflict resolution.Known gaps, tracked as follow-ups rather than widened into this PR
gatewayErrors.ts. Mobile surfacesasError's output directly, so a CGW 502 there shows "Request failed with status 502" — no HTML leak, but a status line rather than the agreed copy.status: 429could be labelledCGW-429. The proper fix is branding the error at the throw site.apps/web/src/utils/rtkQuery.tsremains a parallel generic-copy mechanism ("Something went wrong (502). Please try again…") used by chains and spaces flows. Pre-existing; worth consolidating onto this copy source.Visual summary
flowchart TB G["CGW answers 502 with an HTML error page"] subgraph Before B1["fetchBaseQuery: JSON.parse fails"] --> B2["{ status: 'PARSING_ERROR',<br/>originalStatus: 502,<br/>data: '<html>…nginx…' }"] B2 --> B3["asError: data-is-string branch wins<br/>message = the whole HTML<br/>status = 'PARSING_ERROR'"] B3 --> B4["getHttpStatusFromError → undefined<br/>the real 502 is lost"] B3 --> B5["<pre> in the toast Details<br/>renders raw HTML"] end subgraph After A1["asError: PARSING_ERROR branch first"] --> A2["message = 'Request failed with status 502'<br/>status = 502 (numeric)"] A2 --> A3["getCgwErrorInfo"] A3 -->|"429 / 5xx / 422"| A4["'Something went wrong on our end. Try again.'"] A3 -->|"451"| A5["'This Safe Account is not available.'"] A3 -->|"404"| A6["unmapped — unchanged, deferred"] A4 --> A7["screen: sentence + Error code CGW-502"] A5 --> A7 A4 --> A8["422 only: internal alert → Datadog"] end G --> B1 G --> A1Checklist
asErroris shared